Skip to content

Conversation

@SeRin-Yang
Copy link

No description provided.

Copy link
Collaborator

@Philipp15b Philipp15b left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks! I've left a bunch of comments.

fn execute_swine(dir: &Path, file_path: &Path) {
let swine = "swine-z3";

let find_output = Command::new("find")
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't understand why you'd be using find here? It should be sufficient to just do the call to swine directly.

.output().unwrap();

if cmd_output.status.success() {
println!("{}", String::from_utf8_lossy(&cmd_output.stdout));
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The next step would be to parse the output of SWINE and return a SatResult.

println!("{}", String::from_utf8_lossy(&cmd_output.stdout));
break;
} else {
eprintln!("Failed to execute swine({}) command with status: {}", line, cmd_output.status);
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ideally, return a Result and return an error in this case.

}
}
} else {
eprintln!("Find command execution failed");
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same here.

}
}

fn remove_lines_for_swine(input: &str) -> String {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please add a doc comment saying which lines this removes.

return ProveResult::Proof;
}

let mut smtlib = self.get_smtlib();
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As a next step, add an solver_type attribute of a new enum type SolverType with variants Z3 and SWINE, and then choose here whether to invoke either Z3 or SWINE.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You can do this like we did it for SliceVerifyMethod and the SliceOptions::slice_verify_via option in main.rs.

Ok((SatResult::Unsat, "".to_string()))
} else if first_line.trim().to_lowercase() == "sat" {
let _last_line = lines_buffer.pop_back().ok_or(ProverCommandError::ParseError)?;
if _last_line.contains("SHA") {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks strange and at least requires a comment why that's here.

} else {
Err(ProverCommandError::ParseError)
}
} else {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should only return Unknown if the SAT solver result is actually unknown. If there's a different result than we expected, we must throw an error.

let mut input_buffer: VecDeque<char> = input.chars().collect();
let mut cnt = 0;

while let Some(c) = input_buffer.pop_front() {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe add a comment that you collect the full string for a (possibly multi-line) SMTLIB command by looking for the corresponding closed parenthesis, while counting nested parenthesis.

self.min_level_with_provables.get_or_insert(self.level);
}

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Be sure to have your editor automatically remove trailing spaces. cargo fmt --all should remove these as well, I think.

}
}
SolverType::Z3 => {
let res = match &self.last_result {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All of this logic needs to happen for the SWINE calls as well! This includes returning cached results, checking with assumptions (where did the assumptions go when using SWINE??) and caching results afterwards.

ProveResult::Proof => {}
Ok(ProveResult::Unknown(reason)) => panic!("solver returned unknown ({})", reason),
Ok(ProveResult::Proof) => {},
Err(_) => {}
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why is the error here silently ignored?

z3rro/Cargo.toml Outdated
im-rc = "15"
enum-map = "2.7.3"
itertools = "0.14.0"
smtlib-lowlevel = "0.3.0"
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are we using this still?

@Philipp15b
Copy link
Collaborator

Philipp15b commented May 16, 2025

W.r.t. the "the Err-variant returned from this function is very large" clippy error, I've fixed those on the main branch just now (f8776cb). The other CI errors seem to come from this PR.

prover.add_assumption(&at_most_n_true);
if !slice_vars.is_empty() {
let at_most_n_true = match prover.get_smt_solver() {
SolverType::CVC5 | SolverType::YICES => at_most_k(ctx, at_most_n as i64, active_slice_vars),
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd have the function at_most_k take a SolverType parameter, and have it do the decision of whether to call Bool::pb_le or your own at_most_k implementation.

ProveResult::Counterexample | ProveResult::Unknown(_) => None,
}
});
let res_seed = match check_proof_seed(&all_variables, prover, limits_ref, &seed) {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wait, I don't think this is the same behavior! shrink_block_unsat should call check_proof_seed as before!

ProveResult::Proof | ProveResult::Unknown(_) => false,
}
});
let res_seed = match check_proof_seed(&all_variables, prover, limits_ref, &seed) {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same as before, this is not the same behavior!

Smtlib::from_solver(self.get_solver())
}

pub fn get_smt_solver(&self) -> SolverType {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should be named get_solver_type

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants